Refactored CommitAbility into TryCommitAbility. - #60
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors ability activation and commitment APIs to Try* patterns, adds commit failure routing, exposes transient ability handles, and corrects cost projection.
Changes:
- Introduces
TryCommitAbilityNodeand fallible commit APIs. - Renames activation/grant APIs and adds lingering-handle output.
- Updates cost validation, tests, and documentation.
Reviewed changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
Forge/Statescript/Providers/IAbilityActivationDataProvider.cs |
Adds granted-handle output. |
Forge/Statescript/Providers/AbilityActivationDataProvider.cs |
Uses new Try APIs. |
Forge/Statescript/Properties/AbilityActivatorResolver.cs |
Updates renamed node documentation. |
Forge/Statescript/Properties/AbilityActivator.cs |
Forwards granted handles. |
Forge/Statescript/Nodes/Condition/TryGrantAbilityAndActivateOnceNode.cs |
Renames node and adds handle output. |
Forge/Statescript/Nodes/Condition/TryCommitAbilityNode.cs |
Adds conditional commit node. |
Forge/Statescript/Nodes/Condition/TryActivateAbilityNode.cs |
Uses TryActivate. |
Forge/Statescript/Nodes/Condition/CommitAbilityOperation.cs |
Moves enum to condition namespace. |
Forge/Statescript/Nodes/Action/CommitAbilityNode.cs |
Removes obsolete action node. |
Forge/Effects/Modifiers/Modifier.cs |
Removes old affordability helper. |
Forge/Effects/EffectsManager.cs |
Removes old cost validation path. |
Forge/Effects/Components/GrantAbilityEffectComponent.cs |
Uses TryActivate. |
Forge/Core/EntityAbilities.cs |
Adds Try grant-and-activate APIs. |
Forge/Abilities/AbilityHandle.cs |
Exposes Try activation and commit APIs. |
Forge/Abilities/AbilityBehaviorContext.Data.cs |
Updates API reference. |
Forge/Abilities/Ability.cs |
Adds commit validation and cost projection. |
Forge.Tests/Statescript/Resolvers/AbilityDataResolverTests.cs |
Updates renamed API usage. |
Forge.Tests/Statescript/Resolvers/AbilityActivatorResolverTests.cs |
Tests granted-handle output. |
Forge.Tests/Statescript/Nodes/State/ListenerNodesTests.cs |
Updates activation calls. |
Forge.Tests/Statescript/Nodes/GrantAbilityNodesTests.cs |
Tests renamed node and output. |
Forge.Tests/Statescript/Nodes/Condition/TryCommitAbilityNodeTests.cs |
Covers conditional commits. |
Forge.Tests/Statescript/Nodes/Condition/ExpressionNodeTests.cs |
Updates activation call. |
Forge.Tests/Statescript/Nodes/Action/CommitAbilityNodeTests.cs |
Removes obsolete node tests. |
Forge.Tests/Statescript/GraphAbilityBehaviorTests.cs |
Updates activation calls. |
Forge.Tests/Samples/QuickStartTests.cs |
Updates sample API usage. |
Forge.Tests/Helpers/StatescriptTestHelpers.cs |
Updates test helpers. |
Forge.Tests/Effects/CancelAbilityTagsComponentTests.cs |
Updates activation calls. |
Forge.Tests/Effects/BlockAbilityTagsComponentTests.cs |
Updates activation calls. |
Forge.Tests/Abilities/EntityAbilitiesEventsTests.cs |
Updates event tests. |
Forge.Tests/Abilities/AbilityBehaviorTests.cs |
Tests Try commits and transient handles. |
Forge.Tests/Abilities/AbilitiesTests.cs |
Expands commit and cost validation coverage. |
docs/statescript/resolvers/can-activate-ability-resolver.md |
Updates commit-node link. |
docs/statescript/resolvers/ability-activator-resolver.md |
Documents renamed APIs. |
docs/statescript/nodes/state/grant-ability-node.md |
Updates related-node link. |
docs/statescript/nodes/condition/try-grant-ability-and-activate-once-node.md |
Documents renamed node and output. |
docs/statescript/nodes/condition/try-commit-ability-node.md |
Documents new commit node. |
docs/statescript/nodes/condition/try-activate-ability-node.md |
Documents TryActivate. |
docs/statescript/nodes/condition/README.md |
Updates condition-node index. |
docs/statescript/nodes/action/README.md |
Removes obsolete action listing. |
docs/statescript/nodes/action/grant-ability-permanently-node.md |
Updates related-node link. |
docs/statescript/nodes/action/commit-ability-node.md |
Removes obsolete documentation. |
docs/statescript/ability-integration.md |
Updates integration guidance. |
docs/quick-start.md |
Updates public API examples. |
docs/abilities.md |
Documents Try APIs and cost behavior. |
Suppressed comments (1)
Forge/Core/EntityAbilities.cs:350
- The typed overload has the same shared-grant issue:
IsValidremains true when another grant source owns the ability, even if this activation failed or already ended. This can expose an inactive handle as the transient proc output; require both successful activation and an active handle.
grantedAbility = abilityHandle.IsValid ? abilityHandle : null;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changed
Added
Fixed